-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update: Refactor panel color gradient settings to use a tools panel. Unify color UI. #41091
Update: Refactor panel color gradient settings to use a tools panel. Unify color UI. #41091
Conversation
Size Change: -604 B (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice work @jorgefilipecosta and a special thanks for the example use of batch
! I wasn't aware of its availability. 🙇
In general, this is testing pretty well except for a couple of minor issues.
There is a more significant problem though with switching the color block supports panel to use the updated PanelColorGradientSettings
. Doing so means the color block support controls are no longer rendered via the grouped InspectorControls
slot which enables us to inject other color-related controls into that single panel.
We can revert the color block support to render its controls via the InspectorControls
slot with only a subset of the code removed in this PR so we still benefit from your work here.
I've included a diff below that includes most of the suggestions from my review, reinstates the slot fill rendering for the color block support, fixes a linter issue etc.
Example diff with review suggestions
diff --git a/packages/block-editor/src/components/colors-gradients/dropdown.js b/packages/block-editor/src/components/colors-gradients/dropdown.js
index 5c76ccb6f9..b13e22b353 100644
--- a/packages/block-editor/src/components/colors-gradients/dropdown.js
+++ b/packages/block-editor/src/components/colors-gradients/dropdown.js
@@ -7,11 +7,11 @@ import classnames from 'classnames';
* WordPress dependencies
*/
import {
+ Button,
ColorIndicator,
Dropdown,
FlexItem,
__experimentalHStack as HStack,
- __experimentalItem as Item,
__experimentalToolsPanelItem as ToolsPanelItem,
} from '@wordpress/components';
@@ -45,6 +45,9 @@ const WithToolsPanelItem = ( { setting, children, panelId, ...props } ) => {
{ ...props }
className="block-editor-tools-panel-color-gradient-settings__item"
panelId={ panelId }
+ // Pass resetAllFilter if supplied due to rendering via SlotFill
+ // into parent ToolsPanel.
+ resetAllFilter={ setting.resetAllFilter }
>
{ children }
</ToolsPanelItem>
@@ -70,16 +73,16 @@ const renderToggle = ( settings ) => ( { onToggle, isOpen } ) => {
const toggleProps = {
onClick: onToggle,
className: classnames(
- 'block-editor-tools-panel-color-gradient-settings__dropdown',
+ 'block-editor-panel-color-gradient-settings__dropdown',
{ 'is-open': isOpen }
),
'aria-expanded': isOpen,
};
return (
- <Item { ...toggleProps }>
+ <Button { ...toggleProps }>
<LabeledColorIndicator colorValue={ colorValue } label={ label } />
- </Item>
+ </Button>
);
};
diff --git a/packages/block-editor/src/components/colors-gradients/panel-color-gradient-settings.js b/packages/block-editor/src/components/colors-gradients/panel-color-gradient-settings.js
index 1cc4b65ef7..797a6c9463 100644
--- a/packages/block-editor/src/components/colors-gradients/panel-color-gradient-settings.js
+++ b/packages/block-editor/src/components/colors-gradients/panel-color-gradient-settings.js
@@ -43,7 +43,7 @@ export const PanelColorGradientSettingsInner = ( {
__experimentalIsRenderedInSidebar,
enableAlpha,
} ) => {
- const panelId = useInstanceId( PanelColorGradientSettingsInner )
+ const panelId = useInstanceId( PanelColorGradientSettingsInner );
const { batch } = useRegistry();
if (
isEmpty( colors ) &&
@@ -90,8 +90,6 @@ export const PanelColorGradientSettingsInner = ( {
} );
} }
panelId={ panelId }
- hasInnerWrapper={ true }
- shouldRenderPlaceholderItems={ true } // Required to maintain fills ordering.
__experimentalFirstVisibleItemClass="first"
__experimentalLastVisibleItemClass="last"
>
diff --git a/packages/block-editor/src/components/colors-gradients/style.scss b/packages/block-editor/src/components/colors-gradients/style.scss
index abac92db1b..20c146ee71 100644
--- a/packages/block-editor/src/components/colors-gradients/style.scss
+++ b/packages/block-editor/src/components/colors-gradients/style.scss
@@ -112,3 +112,7 @@
}
}
}
+
+.block-editor-panel-color-gradient-settings__dropdown {
+ width: 100%;
+}
diff --git a/packages/block-editor/src/hooks/color-panel.js b/packages/block-editor/src/hooks/color-panel.js
index 34a8645ea7..d0775abbc4 100644
--- a/packages/block-editor/src/hooks/color-panel.js
+++ b/packages/block-editor/src/hooks/color-panel.js
@@ -6,12 +6,11 @@ import { useState, useEffect } from '@wordpress/element';
/**
* Internal dependencies
*/
-import PanelColorGradientSettings from '../components/colors-gradients/panel-color-gradient-settings';
+import ColorGradientSettingsDropdown from '../components/colors-gradients/dropdown';
import ContrastChecker from '../components/contrast-checker';
import InspectorControls from '../components/inspector-controls';
import useMultipleOriginColorsAndGradients from '../components/colors-gradients/use-multiple-origin-colors-and-gradients';
import { __unstableUseBlockRef as useBlockRef } from '../components/block-list/use-block-props/use-block-refs';
-import { __ } from '@wordpress/i18n';
function getComputedStyle( node ) {
return node.ownerDocument.defaultView.getComputedStyle( node );
@@ -63,24 +62,24 @@ export default function ColorPanel( {
const colorGradientSettings = useMultipleOriginColorsAndGradients();
return (
- <InspectorControls>
- <PanelColorGradientSettings
+ <InspectorControls __experimentalGroup="color">
+ <ColorGradientSettingsDropdown
enableAlpha={ enableAlpha }
+ panelId={ clientId }
settings={ settings }
+ __experimentalIsItemGroup={ false }
__experimentalHasMultipleOrigins
__experimentalIsRenderedInSidebar
{ ...colorGradientSettings }
- title={ __( 'Color' ) }
- >
- { enableContrastChecking && (
- <ContrastChecker
- backgroundColor={ detectedBackgroundColor }
- textColor={ detectedColor }
- enableAlphaChecker={ enableAlpha }
- linkColor={ detectedLinkColor }
- />
- ) }
- </PanelColorGradientSettings>
+ />
+ { enableContrastChecking && (
+ <ContrastChecker
+ backgroundColor={ detectedBackgroundColor }
+ textColor={ detectedColor }
+ enableAlphaChecker={ enableAlpha }
+ linkColor={ detectedLinkColor }
+ />
+ ) }
</InspectorControls>
);
}
diff --git a/packages/block-editor/src/hooks/color.js b/packages/block-editor/src/hooks/color.js
index 52ade28f37..3219e01692 100644
--- a/packages/block-editor/src/hooks/color.js
+++ b/packages/block-editor/src/hooks/color.js
@@ -76,6 +76,60 @@ const hasTextColorSupport = ( blockType ) => {
return colorSupport && colorSupport.text !== false;
};
+/**
+ * Clears a single color property from a style object.
+ *
+ * @param {Array} path Path to color property to clear within styles object.
+ * @param {Object} style Block attributes style object.
+ * @return {Object} Styles with the color property omitted.
+ */
+const clearColorFromStyles = ( path, style ) =>
+ cleanEmptyObject( immutableSet( style, path, undefined ) );
+
+/**
+ * Clears text color related properties from supplied attributes.
+ *
+ * @param {Object} attributes Block attributes.
+ * @return {Object} Update block attributes with text color properties omitted.
+ */
+const resetAllTextFilter = ( attributes ) => ( {
+ textColor: undefined,
+ style: clearColorFromStyles( [ 'color', 'text' ], attributes.style ),
+} );
+
+/**
+ * Clears link color related properties from supplied attributes.
+ *
+ * @param {Object} attributes Block attributes.
+ * @return {Object} Update block attributes with link color properties omitted.
+ */
+const resetAllLinkFilter = ( attributes ) => ( {
+ style: clearColorFromStyles(
+ [ 'elements', 'link', 'color', 'text' ],
+ attributes.style
+ ),
+} );
+
+/**
+ * Clears all background color related properties including gradients from
+ * supplied block attributes.
+ *
+ * @param {Object} attributes Block attributes.
+ * @return {Object} Block attributes with background and gradient omitted.
+ */
+const clearBackgroundAndGradient = ( attributes ) => ( {
+ backgroundColor: undefined,
+ gradient: undefined,
+ style: {
+ ...attributes.style,
+ color: {
+ ...attributes.style?.color,
+ background: undefined,
+ gradient: undefined,
+ },
+ },
+} );
+
/**
* Filters registered block settings, extending attributes to include
* `backgroundColor` and `textColor` attribute.
@@ -414,6 +468,7 @@ export function ColorEdit( props ) {
style?.color?.text
).color,
isShownByDefault: defaultColorControls?.text,
+ resetAllFilter: resetAllTextFilter,
},
]
: [] ),
@@ -435,6 +490,7 @@ export function ColorEdit( props ) {
: undefined,
isShownByDefault:
defaultColorControls?.background,
+ resetAllFilter: clearBackgroundAndGradient,
},
]
: [] ),
@@ -450,6 +506,7 @@ export function ColorEdit( props ) {
clearable: !! style?.elements?.link?.color
?.text,
isShownByDefault: defaultColorControls?.link,
+ resetAllFilter: resetAllLinkFilter,
},
]
: [] ),
diff --git a/packages/block-editor/src/hooks/color.scss b/packages/block-editor/src/hooks/color.scss
index 2b79f03a50..f9b7898f94 100644
--- a/packages/block-editor/src/hooks/color.scss
+++ b/packages/block-editor/src/hooks/color.scss
@@ -20,4 +20,13 @@
row-gap: 0;
}
}
+
+ /**
+ * After converting PanelColorGradientSettings to render as a ToolsPanel
+ * we need to remove the top margin when wrapping inner content due to
+ * rendering via SlotFills.
+ */
+ .block-editor-tools-panel-color-gradient-settings__item.first {
+ margin-top: 0;
+ }
}
diff --git a/packages/block-library/src/social-links/edit.js b/packages/block-library/src/social-links/edit.js
index ed148909ad..f06f2ff42b 100644
--- a/packages/block-library/src/social-links/edit.js
+++ b/packages/block-library/src/social-links/edit.js
@@ -215,16 +215,17 @@ export function SocialLinksEdit( props ) {
__experimentalIsRenderedInSidebar
title={ __( 'Color' ) }
colorSettings={ colorSettings }
- />
- { ! logosOnly && (
- <ContrastChecker
- { ...{
- textColor: iconColorValue,
- backgroundColor: iconBackgroundColorValue,
- } }
- isLargeText={ false }
- />
- ) }
+ >
+ { ! logosOnly && (
+ <ContrastChecker
+ { ...{
+ textColor: iconColorValue,
+ backgroundColor: iconBackgroundColorValue,
+ } }
+ isLargeText={ false }
+ />
+ ) }
+ </PanelColorSettings>
</InspectorControls>
<ul { ...innerBlocksProps } />
</Fragment>
diff --git a/packages/components/src/tools-panel/styles.ts b/packages/components/src/tools-panel/styles.ts
index 715707a100..6dec56f1b5 100644
--- a/packages/components/src/tools-panel/styles.ts
+++ b/packages/components/src/tools-panel/styles.ts
@@ -54,8 +54,7 @@ export const ToolsPanelWithInnerWrapper = ( columns: number ) => {
};
export const ToolsPanelHiddenInnerWrapper = css`
- /* Required to meet specificity requirements */
- &&& > div:not( :first-of-type ) {
+ > div:not( :first-of-type ) {
display: none;
}
`;
Minor Issues
Cover Opacity
With the change to use ToolsPanel
for PanelColorGradientSettings
, the opacity control won't be included within the ToolsPanel
menu and so "reset all" doesn't appear to reset all controls in the panel.
I have some code floating around locally where I tested out moving these Cover color related controls into the color block supports panel. I can create a PR doing just that I've created a draft PR for this ( #41102 ) assuming we revert the change this PR makes that prevents that. It will need some tweaks after this lands though.
Screen.Recording.2022-05-17.at.1.37.43.pm.mp4
Contrast Checker Outside Panel
After applying this PR, the contrast checker for the Social Links icon colors stretches the full width of the sidebar. No spacing or margin around it.
This can be fixed by rendering the contrast checker as a child of the PanelColorSettings
in the Social Links block (change is also in the above diff).
packages/block-editor/src/components/colors-gradients/dropdown.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/colors-gradients/dropdown.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/colors-gradients/panel-color-gradient-settings.js
Outdated
Show resolved
Hide resolved
packages/block-editor/src/components/colors-gradients/panel-color-gradient-settings.js
Outdated
Show resolved
Hide resolved
f8988ac
to
e1ec461
Compare
Hi @aaronrobertshaw, thank you for the review and for providing a patch with your suggestions. I think your suggestions are in the right direction and I applied your patch 👍 Is there any other change you think we should do? I think your PR enhancing things on the cover block is nice and we should merge it as a follow-up. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've re-tested this after the latest updates and it's working well for me 👍
✅ General color block supports still display and function correctly
✅ Cover block color panel works (opacity reset issue to be addressed via #41102)
✅ Social Links contrast checker displays within the related panel now
✅ Navigation block color controls work as before
✅ Tested injecting color controls into block supports panel via #41102
Is there any other change you think we should do?
I believe this PR covers all we need for now. Anything else should be able to be addressed via a follow-up.
e1ec461
to
ed7c83c
Compare
Thank you a lot for the help in this PR @aaronrobertshaw! |
Looking at these latest changes, I can see This makes using it anywhere else impossible - I'd like to use this in a regular panel, in a custom block. My code is working with WP 5.9 and 6.0, but trying after this update nothing shows (naturally). Is it completely necessary to make the change preventing this from working outside a Would it make more sense to name the component (or offer a secondary version, wrapped in Its a really useful component and would love to carry on using it. |
This PR updates PanelColorGradientSettings to use the tools panel and makes the color support hook use PanelColorGradientSettings again.
PanelColorGradientSettings keeps almost the same API, only gets a new option isShownByDefault setting that allows one to hide a color option by default.
This refactor makes all the blocks using custom color colors core (cover, navigation, etc...) and the third-party blocks have the same color UI as the blocks relying on the color support hook unifying all the color picking interface.
With this change, we are also able to remove a considerable amount of code.
How?
I verified I could pick an overlay color for the cover, a color for a paragraph, and colors for navigation, remove the colors, and reset the color.